fix(review): reduce secret-scan false positives and surface finding locations by JSONbored · Pull Request #3178 · JSONbored/loopover

JSONbored · 2026-07-04T19:09:41Z

Summary

Fixes a false-positive hard block from the generic_secret_assignment rule: a captured value made
entirely of lowercase words joined by hyphens (2+ segments, e.g. installation-token — the test-fixture
mock token used 351+ times across this repo's own test suite) is now treated as a non-secret. Real
secrets/tokens are essentially always alphanumeric, mixed-case, or base64/hex, never a pure
lowercase-hyphenated phrase, so this exclusion stays narrow (requires ≥1 hyphen) and doesn't broaden into
excluding arbitrary single lowercase words. This is the confirmed root cause of PR fix(review): key the durable CI-state cache on resolved required contexts #3036's wrongful
hard-block (test/unit/queue.test.ts's Response.json({ token: "installation-token" }) fixture).
Fixes the second part of fix(review): reduce false positives in the gate's hard-blocking secret scan #3041: a hard-blocking secret_leak finding previously carried no file:line
location at all, forcing a maintainer to re-derive it from the whole diff. Added
scanDiffForSecretsWithLocations in src/review/secrets-scan.ts, which walks a buildSecretScanDiff-shaped
diff line by line (tracking file headers, hunk headers, and +/-/context line types) and returns each
pattern hit with its file path and 1-based line number in the new/post-change file (or line: 0 for a
secret-shaped filename on an added/renamed file header, preserving the previous header-scanning behavior).
secretLeakFinding in src/review/safety.ts now calls this directly on the raw diff (no more
pre-filtering to an added-only text blob) and appends up to 5 path:line locations to the finding's
detail, noting how many more were omitted beyond that cap.
The pattern-matching logic (SECRET_PATTERNS + hasGenericSecretAssignment) was factored into one shared
matchedKindsIn helper that both scanForSecrets (unchanged signature/behavior) and the new
scanDiffForSecretsWithLocations delegate to, so there is exactly one place the pattern list is applied.
Out of scope (per the issue, explicitly "Consider"-worded): an owner/maintainer override/acknowledgment
path for a confirmed false positive. That's a separate, larger design decision; left as a follow-up rather
than built here.

Scope

The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

Also ran (not template-listed, but part of this repo's full local gate): npm run db:migrations:check,
npm run db:schema-drift:check, npm run cf-typegen:check, npm run selfhost:validate-observability,
npm run ui:test, npm run ui:version-audit, npm run rees:test (677 passed) — all green.

If any required check was skipped, explain why:

npm run selfhost:env-reference:check (part of the local test:ci aggregator, but not part of the
validate-code GitHub Actions job) fails on a clean origin/main checkout with no changes from this PR —
apps/gittensory-ui/src/lib/selfhost-env-reference.ts drifted from a prior, unrelated merge. Verified via
git stash that this failure is present before any of this PR's changes are applied. This PR touches
neither the self-host env-var surface nor that generated file, so regenerating it here would be out of
scope; flagging as a pre-existing, separate issue rather than fixing it in this PR.

Safety

No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
API/OpenAPI/MCP behavior is updated and tested where needed.
UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

This is a deterministic, security-relevant scanner change, so extra care was taken to verify it doesn't
regress detection of real secrets:

All pre-existing test/unit/secrets-scan.test.ts and test/unit/safety-wiring.test.ts tests pass
unmodified — including every format-specific pattern (github_token, github_pat, aws_access_key,
slack_token, google_api_key, gitlab_token, npm_token, stripe_secret_key, sendgrid_key,
huggingface_token, jwt, private_key_block) and the existing generic_secret_assignment positive case
(sk_live_ + mixed-case/digit value), none of which are touched by the new exclusion.
New regression guard: a mixed-case/digit-bearing fake secret (same shape as the existing positive test)
still flags after the fix, proving the new lowercase-hyphenated-compound exclusion is narrow and doesn't
broaden past its intended shape.
New test proving a single lowercase word with no hyphen is unaffected by the new exclusion (still flags),
confirming the regex specifically requires 2+ hyphen-joined segments.
The format-specific SECRET_PATTERNS regexes (github_token, aws_access_key, etc.) are completely
untouched — the fix only changes isPlaceholderSecretValue, which exclusively gates the
generic_secret_assignment heuristic path.
secretLeakFinding's existing "scans only added lines / added-or-renamed file headers, never
removed/context lines" behavior is preserved exactly — now implemented via
scanDiffForSecretsWithLocations's own line-type handling instead of a pre-filter — and all of that
behavior's original tests pass unmodified, plus a new explicit test that a removed line's secret-shaped
content never appears in the finding.

UI Evidence

N/A — backend-only change (src/review/**), no UI/frontend/docs/extension surface touched.

Notes

Root cause and fix scope were pre-investigated and confirmed empirically against PR fix(review): key the durable CI-state cache on resolved required contexts #3036's real diff
before this PR was written.

…ocations Exclude lowercase-hyphenated word compounds (e.g. the "installation-token" test fixture used 351+ times in this repo's own suite) from the generic secret-assignment heuristic, and surface file:line locations in the secret_leak finding so a hard-blocking match can be verified without re-deriving it from the whole diff. Fixes #3041

superagent-security · 2026-07-04T19:10:34Z

Superagent didn't find any vulnerabilities or security issues in this PR.

codecov · 2026-07-04T19:15:39Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.72%. Comparing base (ee4e8ef) to head (fbc78b0).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3178   +/-   ##
=======================================
  Coverage   94.72%   94.72%           
=======================================
  Files         269      269           
  Lines       29619    29658   +39     
  Branches    10806    10814    +8     
=======================================
+ Hits        28056    28095   +39     
  Misses        917      917           
  Partials      646      646

Files with missing lines	Coverage Δ
src/review/safety.ts	`100.00% <100.00%> (ø)`
src/review/secrets-scan.ts	`100.00% <100.00%> (ø)`

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

loopover-orb · 2026-07-04T19:16:14Z

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

_{Review updated: 2026-07-04 19:13:48 UTC}

4 files · 1 AI reviewer · 1 blocker · readiness 100/100 · CI green · clean

⏸️ Suggested Action - Manual Review

Possible leaked secret in the diff (generic_secret_assignment) — Remove the secret from the diff, rotate the exposed credential, then re-run the gate.

Review summary
The change correctly moves secret scanning to a structured diff walk so `secret_leak` findings can include file:line locations while preserving the prior added-line-only behavior. The lowercase-hyphenated placeholder exclusion is applied at the captured value source, and the tests cover the real `scanForSecrets`/`secretLeakFinding` paths rather than fabricating an unreachable payload. I do not see a reachable correctness blocker in the provided diff.

Nits — 6 non-blocking

nit: src/review/secrets-scan.ts:169 treats any non-header/non-removed line as a new-file context line, so malformed hunk headers like the existing test fixture `@@` or metadata lines inside a patch can shift reported locations; consider only incrementing for `line.startsWith(" ")` and leaving separators/metadata neutral.
nit: src/review/secrets-scan.ts:72 has a nested-looking lowercase-hyphen pattern; it is delimiter-bounded by hyphens, but a short comment or a split-based helper would make the intentional non-ReDoS shape clearer for future reviewers.
nit: test/unit/safety-wiring.test.ts:514 covers line attribution on a normal hunk, but there is no direct unit coverage for `scanDiffForSecretsWithLocations` returning multiple kinds or deduping repeated locations through `secretLeakFinding`.
src/review/secrets-scan.ts:169: change the fallback increment to only count real context lines, e.g. `if (line.startsWith(" ")) currentNewLine += 1;`, so separators and patch metadata cannot perturb later locations in malformed or hand-built diffs.
src/review/secrets-scan.ts:72: consider replacing the regex with `value.includes("-") && value.split("-").every((part) => /^[a-z]+$/.test(part))` to make the narrow lowercase-compound exclusion self-evident.
PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

Concerns raised — review before merging

Possible leaked secret in the diff (generic_secret_assignment) — Remove the secret from the diff, rotate the exposed credential, then re-run the gate.

Signal	Result	Evidence
Code review	❌ 1 blocker	1 reviewer
Linked issue	✅ Linked	#3041
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 60 registered-repo PR(s), 50 merged, 442 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 60 PR(s), 442 issue(s).
Gate result	❌ Blocking	Repo-configured hard blocker found.

Review context

Author: JSONbored
Role context: owner (maintainer lane)
Public audience mode: oss maintainer
Lane context: Repository registration is not available in the local Gittensory cache.
Public profile languages: not available
Official Gittensor activity: 60 PR(s), 442 issue(s).
PR-specific overlap: none found.

Contributor next steps

Treat this as maintainer-lane context rather than normal contributor-lane activity.
No action.

Signal definitions

Related work = same linked issue, overlapping active PRs, or title/path similarity.
Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

Re-run Gittensory review

) PR #5346 (a resubmission of #5341) was auto-closed over two inert test-fixture strings that matched the generic_secret_assignment keyword-plus-quoted-value SHAPE but weren't real credentials -- the same heuristic has now caused at least eight prior false-positive incidents (#2613, #3178, #3673, #3866, #4587, #4733, plus several fixture-rewording commits), each patched by narrowing an allowlist rather than fixing the underlying design. REES's own copy of this rule already rates it "medium confidence" ("catches real keys but also the occasional long opaque non-secret"), and content-lane/security-scan.ts's own header states the design principle this violated: a gate that auto-closes with no human queue may only hard-close on a signal unambiguous enough that a false positive is essentially impossible. Split generic_secret_assignment out of HARD_SECRET_KINDS into a new ADVISORY_ONLY_SECRET_KINDS: it still surfaces (a warning-severity possible_secret_assignment finding / a "manual" content-lane verdict), but never auto-blocks or auto-closes on its own. Concrete credential formats (github_token, aws_access_key, private_key_block, ...) are unaffected and remain unconditional hard blockers. Also add a structural placeholder heuristic (looksLikeDescriptive PlaceholderPhrase, mirrored in REES): a value with 5+ lowercase-only hyphen/underscore segments containing an English function word reads as written prose describing the value, not a credential or a chosen passphrase -- this independently resolves both PR #5346 literals without weakening detection of a genuine human-chosen passphrase like "correct-horse-battery-secret" (no function words, by design).

JSONbored mentioned this pull request Jul 4, 2026

fix(review): reduce false positives in the gate's hard-blocking secret scan #3041

Closed

JSONbored self-assigned this Jul 4, 2026

JSONbored added this to Self-Hosted Review Stack (LoopOver Orb) Jul 4, 2026

JSONbored added this to the Review-stack enhancements (epic #1953) milestone Jul 4, 2026

github-project-automation Bot moved this to Todo in Self-Hosted Review Stack (LoopOver Orb) Jul 4, 2026

loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 4, 2026

loopover-orb Bot added the gittensor label Jul 4, 2026

JSONbored merged commit 11a061d into main Jul 4, 2026
10 checks passed

JSONbored deleted the fix/secret-scan-false-positive-3041 branch July 4, 2026 19:21

github-project-automation Bot moved this from Todo to Done in Self-Hosted Review Stack (LoopOver Orb) Jul 4, 2026

JSONbored mentioned this pull request Jul 12, 2026

fix(review): stop generic_secret_assignment from auto-closing PRs #5370

Merged

7 tasks

JSONbored mentioned this pull request Jul 12, 2026

fix(review): hard-block generic secret assignments #5493

Closed

Navigation Menu

Uh oh!

fix(review): reduce secret-scan false positives and surface finding locations - #3178

fix(review): reduce secret-scan false positives and surface finding locations#3178
JSONbored merged 1 commit into
mainfrom
fix/secret-scan-false-positive-3041

JSONbored commented Jul 4, 2026

Uh oh!

superagent-security Bot commented Jul 4, 2026

Uh oh!

codecov Bot commented Jul 4, 2026 •

edited

Loading

Uh oh!

loopover-orb Bot commented Jul 4, 2026 •

edited

Loading

⏸️ Gittensory review result - manual review recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

JSONbored commented Jul 4, 2026

Summary

Scope

Validation

Safety

UI Evidence

Notes

Uh oh!

superagent-security Bot commented Jul 4, 2026

Uh oh!

codecov Bot commented Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

loopover-orb Bot commented Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⏸️ Gittensory review result - manual review recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented Jul 4, 2026 •

edited

Loading

loopover-orb Bot commented Jul 4, 2026 •

edited

Loading